Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

Add WebP support to psammead-image component #4606

Merged
merged 25 commits into from
Dec 13, 2021
Merged

Add WebP support to psammead-image component #4606

merged 25 commits into from
Dec 13, 2021

Conversation

jroebu14
Copy link
Contributor

@jroebu14 jroebu14 commented Dec 3, 2021

Part of bbc/simorgh#9628

Overall change:

  • Updates the psammead-image component to use a <picture> element with 2 sources. 1 for WebP srcsets and 1 fallback srcset for JPG/PNG.
  • Adds similar fallback handling to the AMP version of the image component
  • Bumps psammead-image and psammead-media-player major versions

Code changes:

  • Added <picture> tag to the psammead-image component
  • Added <amp-img fallback.../> nested component to handle image format fallback in AMP
  • Updated various test sets and Storybook stories that use the image component to reflect the new fallbackSrcset prop

  • (BBC contributors only) This PR follows the repository use guidelines
  • I have assigned myself to this PR and the corresponding issues
  • Automated jest tests added (for new features) or updated (for existing features)
  • This PR requires manual testing

@amoore108 amoore108 changed the title webp initial investigation work Add WebP support to psammead-image component Dec 9, 2021
@amoore108 amoore108 marked this pull request as ready for review December 9, 2021 15:48
@amoore108 amoore108 self-assigned this Dec 9, 2021
Copy link
Contributor Author

@jroebu14 jroebu14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👏

Even though you did 99% of the work @amoore108 GitHub won't let me approve because I'm the author of the PR. Has my approval anyway 🙂

@amoore108
Copy link
Contributor

Nice 👏

Even though you did 99% of the work @amoore108 GitHub won't let me approve because I'm the author of the PR. Has my approval anyway 🙂

Haha no worries! Thanks for the pairing at the start, most of the changes I did were just test fixes. I'll approve it on my end.

@@ -19,13 +29,15 @@ AmpImg.propTypes = {
sizes: string,
src: string.isRequired,
srcset: string,
fallbackSrcset: string,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new prop may warrant a doc update to readme.md

Copy link
Contributor

@ryanmccombe ryanmccombe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@amoore108 amoore108 merged commit dc0eb83 into latest Dec 13, 2021
@amoore108 amoore108 mentioned this pull request Dec 13, 2021
4 tasks
@amoore108 amoore108 deleted the webp branch April 13, 2022 12:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants